Add examples - #1
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds example code demonstrating three Python packaging scenarios for Deephaven applications: a library-only package, a CLI-only package, and a combined package. The examples show how to structure Python packages using modern packaging standards with pyproject.toml, implement CLI tools with Click, and create reusable Deephaven query functions.
Changes:
- Added three complete example packages (my_dh_library, my_dh_cli, my_dh_toolkit) demonstrating different packaging approaches
- Included sample CSV data files for testing the examples
- Updated main README with comprehensive documentation on package structure, usage patterns, and troubleshooting
Reviewed changes
Copilot reviewed 34 out of 38 changed files in this pull request and generated 29 comments.
Show a summary per file
| File | Description |
|---|---|
| my_dh_library/* | Library-only package with reusable Deephaven query and utility functions |
| my_dh_cli/* | CLI-only package with command-line tools for CSV processing |
| my_dh_toolkit/* | Combined package with both library functions and CLI tools |
| data/* | Sample CSV files for testing the example packages |
| README.md | Comprehensive documentation covering all three packaging scenarios and usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Python Packaging with Deephaven | ||
|
|
||
| This example demonstrates how to create and deploy Python packages that use Deephaven. It shows you how to package both command-line tools and reusable libraries using modern Python packaging standards. | ||
| This repository demonstrates how to create and deploy Python packages that use Deephaven. It shows three complete packaging scenarios following the official [Python Packaging User Guide](https://packaging.python.org/en/latest/guides/writing-pyproject-toml/) recommendations. |
There was a problem hiding this comment.
I'm just starting, so I don't yet have a full view on scope. This wording is imprecise on if we are talking about Core or Enterprise or both.
There was a problem hiding this comment.
Wording here is weird. Is "your" really the proper voice for this stuff? Assess the rest of the document for voice.
| Command-line tools for processing data with Deephaven. | ||
| Command-line tool without exposing library code. |
There was a problem hiding this comment.
At this point, I'm concerned that this doc used AI too heavily without a solid human review. Is this change really a step forward or just more confusing?
| **Usage:** | ||
| ```python | ||
| # Use within a Python session with server running | ||
| from my_dh_cli.cli import my_dh_query | ||
| result = my_dh_query("input_data.csv", verbose=True) | ||
| ``` |
There was a problem hiding this comment.
This usage seems inconsistent with the naming and above description. Above docs and the name imply CLI (aka command line tool). This example is not an example of a command line tool and is just a python library.
| Both reusable library code and command-line tools in one package. | ||
| Both reusable library code and command-line tools. |
There was a problem hiding this comment.
I don't know what this is supposed to mean.
| my_dh_toolkit/ | ||
| ├── src/ | ||
| │ └── my_dh_package/ | ||
| │ └── my_dh_toolkit/ | ||
| │ ├── __init__.py | ||
| │ ├── __main__.py | ||
| │ ├── cli.py | ||
| │ ├── processor.py | ||
| │ ├── queries.py | ||
| │ └── utils.py | ||
| ├── pyproject.toml | ||
| └── README.md |
There was a problem hiding this comment.
This makes me wonder where this example is going. This looks like it includes all of the files from the other packages. If that is the case, why? What are we trying to illustrate?
There was a problem hiding this comment.
I"m going to stop my review here because either I don't understand things, or this file and / or project need significant work.
There was a problem hiding this comment.
I did a quick scan of the file in regular, non-diff view, and I can't tell what the purpose or story of the repo are.
There was a problem hiding this comment.
This PR contains TONS of stuff that should not be checked in. I suspect that it does not have a proper python .gitignore file. I expect to see stuff like this in the .gitignore.
# Distribution / packaging
*.egg
*.egg-info/
.eggs/
This would eliminate all of these *.egg-info dirs. __pycache__ should also not be checked in. I suspect the build dirs should also be excluded. There are tons of replicated files that indicate many config and/or design problems.
I can't even review the project for structure or design because I can't tell the signal from the noise. The repeated files have a code smell, so I do have design concerns.
The accompanying doc will be in a deephaven-core PR.